While these Higher Order Reducers are very clever, I wonder if they are too clever for maintainability :).

withReturnUndefinedOnUnhandledDefault seems very Java to me (aka I have no idea what it does in reading it).

Is it better or worse than com.seriouscompany.business.java.fizzbuzz.packagenamingpackage.interfaces.strategies. SystemOutFizzBuzzOutputStrategy ? 😄

I ended up removing withReturnUndefinedOnUnhandledDefault. It was needed because while ifMatchesAction is meant to allow the reducer to be run conditionally, we must still handle the initialization case. The other option is to provide a default value as an argument of ifMatchesAction, but this felt redundant when the reducer should be able to handle this on its own. Instead, in 623bad6 I remove support for optional query from getQueryParts and test for the property in the action for transforming via replaceAction. Thus, we get the same end-result.

Do you think it's better to track the requesting state closely to the data or better in a separate "network requests" reducer. I've used mainly the first option for a long time but lately, I tend to think that the second one might be better as it's more generic and also speaks to the fact that these requests are "optional" in an offline mode.

Do you think it's better to track the requesting state closely to the data or better in a separate "network requests" reducer. I've used mainly the first option for a long time but lately, I tend to think that the second one might be better as it's more generic and also speaks to the fact that these requests are "optional" in an offline mode.

I thought about this a bit and while I tend to agree that requests doesn't need to be tied so deeply to the items / queries, in practice if we try to separate them we'd end up duplicating much of the same logic anyways. As implemented, they're at least separate so far as being their own individual reducers (earlier iterations were combined into one). The behavior of getQueryParts relies on the fact that we generate a stable key which does not include pagination details. To track separately, we'd need a stable key with the pagination information. I see a few options here:

• Manually concatenate page and perPage back into the stable key string in requests state
• Replace getQueryParts with a simpler getStableKey, and for cases where we need to omit pagination details, do so before passing the object to getStableKey
• Avoid building in pagination awareness into queried-data altogether. Just treat each query as distinct from one another, even on pagination arguments.

On reflection, the last of these is tempting, and could simplify things overall. The idea with the current implementation is that queries are considered the same ignoring pagination information.

There's also the question of: How much does "queried-data" make sense as a thing separate from the REST API? In your offline optional requests system you have in mind, would we still have concepts of querying by ?order=desc&page=2 ?

Do you think this should be memoized to avoid creating a new array on each call?

One issue with memoization is the same as described at #6360 (comment) . For example, consider:

export default withSelect( ( select ) => {
	const { getCategories } = select( 'core' );

	return {
		categories: getCategories( { order: 'desc' } ),
	};
} )( CategoriesBlock );

In this case, we'd encounter a memory leak in that each object reference would be strictly non-equal.

The getQueryParts function gets around this by using the withWeakMapCache helper, which is designed to alleviate this by caching only as long as the object reference is in scope, otherwise allowing garbage collection to occur. Unfortunately that's only effective where we have a single object argument, not two.

We could perhaps keep a local WeakMap cache in scope, and do memoization manually.

Can we have a specific memoization here where we compare the first argument using strict equality (state) and the second one using deep equality (using EquivalentKeyMap maybe)?

Can we have a specific memoization here where we compare the first argument using strict equality (state) and the second one using deep equality (using EquivalentKeyMap maybe)?

Was able to get this idea working in c502dfc.

I had some trouble to understand the exact purpuse of this HoR even with the description here. I think it's the equivalent of a "pure" Higher Order Reducer right?

I had some trouble to understand the exact purpuse of this HoR even with the description here. I think it's the equivalent of a "pure" Higher Order Reducer right?

While it's since been removed, the original purpose was not for purity, but rather to avoid the case where in state initialization, the reducer would be run, and since query was optional for getQueryParts, the initial state would end up looking like:

{
	"items": {},
	"queries": {
		"": {
			"itemIds": null,
			"requestingPageByPerPage": {},
		}
	}
}

...Where it would mistake the initialization as the default (empty) query.

Any specific reason to introduce the constants. Not that I'm strongly against them but I do wonder about having some actions defined as constant and others not.

Any specific reason to introduce the constants. Not that I'm strongly against them but I do wonder about having some actions defined as constant and others not.

It was implemented in the mindset of being an independent module utility, with prefixes on the action types to avoid potential conflicts. In earlier iterations, I had the consumer listening for the particular action types, where I didn't want them to be concerned with said prefixes. Since now only the internal reducer is concerned, we could just replace them with strings. Ultimately, I don't consider these files to be part of core-data or the data module patterns whatsoever, so the conventions don't overlap as much as it might appear on the surface.

Maybe a bit premature to modularize this, but having reimplemented effectively the same thing on many separate occasions, I was motivated toward something more universal 😅